[POS Refunds] Order details button #16414
Conversation
|
|
|
Version |
iamgabrielma
left a comment
iamgabrielma
left a comment
There was a problem hiding this comment.
I’m following the inner padding of the native Menu element in the iOS SDK, which is slightly narrower than the design. This approach aligns with the Apple Human Interface Guidelines and simplifies the code by relying on the native component.
If this is different from the design let's be sure we align with Wagner to update it as well, so there is no discrepancies later when testing or with Android
I also see no ellipsis on the designs from WOOMOB-1786, is there a different Figma file I should be looking to?
| // MARK: - Actions | ||
| private extension POSOrderDetailsView { | ||
| enum POSOrderDetailsAction: Identifiable, CaseIterable { | ||
| enum POSAction: Identifiable, CaseIterable { |
There was a problem hiding this comment.
Is this rename necessary? It makes the POSAction too generic for what is doing, I'd revert it to be POSOrderDetailsAction or something specific to actions to this view. Specially when reaching to .priority or .isAvailable() props they lose readability unless we look into the implementation.
|
|
||
| func available(for order: POSOrder) -> Bool { | ||
| func isAvailable(for order: POSOrder, flags: POSFeatureFlagProviding) -> Bool { | ||
| guard order.status == .completed else { return false } |
There was a problem hiding this comment.
This check breaks the current behavior of showing the "send receipt" button for refunded orders, since will return early if the order is not completed. It should show like this:
| EmptyView() | ||
| } else { | ||
| HStack(spacing: POSSpacing.large) { | ||
| let primary = actions[0] |
There was a problem hiding this comment.
Let's be explicit on which action is triggering rather than rely on array order, using the specific enum case or others.
| let overflow = actions.dropFirst() | ||
| if !overflow.isEmpty { | ||
| Menu { | ||
| ForEach(Array(overflow)) { action in |
There was a problem hiding this comment.
I'm not sure to understand this bit, could you clarify what it does? Why are we dropping the first result when rendering the menu options?
There was a problem hiding this comment.
Not necessarily for this PR, but we should move this logic either up to the model or to a helper so can be tested that we're rendering the correct buttons for all order status cases with feature flag on/off.
Part of WOOMOB-1786
Description
With this PR we add the refunds button to the order details and move the email action to the ellipsis menu.
Notes:
Test Steps
Screenshots
RELEASE-NOTES.txtif necessary.